Switch to first-class aggregate support in pgx #62
Conversation
JamesGuthrie
force-pushed
the
jg/first-class-aggs-2
branch
4 times, most recently
from
13f1301 to
7e25a80
Compare
| #[pg_aggregate] | ||
| impl Aggregate for PromIncrease { | ||
| type State = Option<GapfillDeltaTransition>; | ||
| type Args = ( |
There was a problem hiding this comment.
It's unfortunate that this trait requires an extra allocation just to pack the arguments into a tuple in order to call the state function, but I guess that was a tradeoff they were forced to make. I wonder what the impact of that is in a hot aggregate.
There was a problem hiding this comment.
Definitely cleaner! I think we'll want to try and benchmark the overhead of wrapping arguments in a tuple, since I could see that potentially being an issue in hot aggregates, but we'll have to concoct a decent benchmark setup for that (we probably should be thinking about that anyway, especially with performance being one of our main objectives this year).
Rust does have facilities for that, but we may have to upstream some support in pgx for it, since it would need to build on top of the pgx infra, just like how testing does.
JamesGuthrie
force-pushed
the
jg/first-class-aggs-2
branch
5 times, most recently
from
2c75f82 to
496c7d6
Compare
JamesGuthrie
force-pushed
the
jg/first-class-aggs-2
branch
4 times, most recently
from
0143dc8 to
73025eb
Compare
JamesGuthrie
force-pushed
the
jg/first-class-aggs-2
branch
from
73025eb to
50e73fe
Compare
`#[pgx(sql = false)]` suppresses SQL generation for this object, so the generated SQL is incomplete. We will use this SQL-suppression, but it's missing some pieces which still need to be developed.
JamesGuthrie
force-pushed
the
jg/first-class-aggs-2
branch
from
50e73fe to
03a412e
Compare
- Pgx automatically handles in_aggregate_context, so we can remove some (now) unused code. - Because vector_selector's state type is no longer INTERNAL, we don't need to provide (de)serialization functions. - We have removed backwards-compatibility wrappers for aggregates.
JamesGuthrie
force-pushed
the
jg/first-class-aggs-2
branch
from
03a412e to
153a27e
Compare
Note this is a WIP, and removes backwards-compatibility functions.
Part of #61